Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: switch from @synonymdev/react-native-keychain to react-native-keychain-manager #2279

Closed
wants to merge 1 commit into from
Closed

refactor: switch from @synonymdev/react-native-keychain to react-native-keychain-manager #2279

wants to merge 1 commit into from

Conversation

coreyphillips
Copy link
Collaborator

Description

  • Replaces @synonymdev/react-native-keychain dependency with react-native-keychain-manager.

Type of change

  • Refactoring (improving code without creating new functionality)

Tests

  • No test

QA Notes

We'll want to ensure that both iOS & Android apps using the previous implementation migrate over smoothly to the new implementation.

@coreyphillips
refactor: switch from @synonymdev/react-native-keychain to react-nati…
6a71ee6 
…ve-keychain-manager

Replace @synonymdev/react-native-keychain dependency with react-native-keychain-manager.
@coreyphillips coreyphillips self-assigned this Oct 2, 2024
@socket-security Socket Security
Copy link

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/[email protected] None 0 250 kB mazurdorian

🚮 Removed packages: npm/@synonymdev/[email protected]

View full report↗︎

limpbrains
limpbrains requested changes Oct 3, 2024
View reviewed changes
Copy link
Collaborator

@limpbrains limpbrains left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jest mock file is missing, I think we can reuse
mocks/@synonymdev/react-native-keychain/index.ts

@coreyphillips
Copy link
Collaborator Author

Might want to put a pin in this for now. Looking at recent PR's for v9.0.0 that include the UI freeze we're experiencing here.

@coreyphillips coreyphillips marked this pull request as draft October 3, 2024 13:16
@DorianMazur
Copy link

DorianMazur commented Oct 3, 2024 •
edited
Loading

Hi, I'm the author of react-native-keychain-manager and a maintainer of react-native-keychain. I'll be adding support for AndroidDataStore in react-native-keychain soon. :)

I created react-native-keychain-manager because I lost contact with @oblador and couldn’t merge pull requests, but that issue has been resolved.

@coreyphillips
Copy link
Collaborator Author

Hi, I'm the author of react-native-keychain-manager and a maintainer of react-native-keychain. I'll be adding support for AndroidDataStore in react-native-keychain soon. :)

I created react-native-keychain-manager because I lost contact with @oblador and couldn’t merge pull requests, but that issue has been resolved.

Hey @DorianMazur, thanks for the heads up! We'll keep an eye out and resume testing once AndroidDataStore support is merged.

@DorianMazur
Copy link

@coreyphillips 9.2.0 is ready! Also consider using AES_GCM or AES_GCM_NO_AUTH, because AES_CBC is now deprecated.

@pwltr
Copy link
Collaborator

pwltr commented Nov 25, 2024

PR to switch back to upstream react-native-keychain v9.2.2 and AES_GCM_NO_AUTH here

@pwltr pwltr closed this Nov 25, 2024
@pwltr pwltr deleted the keychain-manager branch November 25, 2024 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@limpbrains limpbrains limpbrains requested changes

@pwltr pwltr Awaiting requested review from pwltr

Assignees

@coreyphillips coreyphillips

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@coreyphillips @DorianMazur @pwltr @limpbrains